Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

components/Onboarding/OnboardingNavButtons.tsx (1)

3-8: Consider exporting the props interface.

Per coding guidelines, component prop types should be exported with explicit ComponentNameProps naming convention for better reusability and documentation.

♻️ Suggested change

-interface OnboardingNavButtonsProps {
+export interface OnboardingNavButtonsProps {
   onBack: () => void;
   onNext: () => void;
   nextLabel?: string;
   nextDisabled?: boolean;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingNavButtons.tsx` around lines 3 - 8, The props
interface OnboardingNavButtonsProps should be exported so other modules can
import and reuse it; update the declaration of the interface
OnboardingNavButtonsProps to be exported (e.g., add the export keyword before
the interface) and ensure any local usages/imports remain valid after making the
interface exportable.

components/Onboarding/onboardingRoleConfig.ts (1)

70-80: Consider referencing the existing "other" role to avoid duplication.

The fallback object in getRoleConfig duplicates the "other" role definition. Reference the existing config to stay DRY.

♻️ Suggested refactor

 export function getRoleConfig(roleId: string | undefined): RoleConfig {
-  return (
-    ROLE_CONFIG_MAP[roleId ?? ""] ?? {
-      id: "other",
-      label: "Other",
-      icon: "✨",
-      description: "",
-      companyLabel: "Company",
-      artistPlaceholder: "Search for an artist…",
-    }
-  );
+  return ROLE_CONFIG_MAP[roleId ?? ""] ?? ROLE_CONFIG_MAP["other"];
 }

Note: This assumes "other" will always exist in ONBOARDING_ROLES. If that's not guaranteed, add a runtime check or keep the inline fallback.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/onboardingRoleConfig.ts` around lines 70 - 80,
getRoleConfig currently returns an inline "other" object as a fallback which
duplicates the existing role; change it to reference the canonical entry in
ROLE_CONFIG_MAP (e.g., use ROLE_CONFIG_MAP["other"]) as the default, and if
"other" may not always exist add a runtime guard that falls back to a minimal
inline object only when ROLE_CONFIG_MAP["other"] is undefined; update
getRoleConfig to use ROLE_CONFIG_MAP[roleId ?? ""] ?? ROLE_CONFIG_MAP["other"]
(with the extra guard if needed).

components/Onboarding/OnboardingStepDots.tsx (2)

4-11: Unused label field in STEPS array.

Each step includes a label property that's never rendered. Either remove the unused field to keep the code lean, or render the labels to provide better user context.

♻️ Option A: Remove unused labels

-const STEPS: { id: OnboardingStep; label: string }[] = [
-  { id: "role", label: "Role" },
-  { id: "context", label: "You" },
-  { id: "artists", label: "Artists" },
-  { id: "connections", label: "Connect" },
-  { id: "pulse", label: "Pulse" },
-  { id: "tasks", label: "Tasks" },
-];
+const STEPS: OnboardingStep[] = [
+  "role", "context", "artists", "connections", "pulse", "tasks"
+];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingStepDots.tsx` around lines 4 - 11, The STEPS
constant contains an unused label field (const STEPS: { id: OnboardingStep;
label: string }[]) in OnboardingStepDots.tsx; either remove the label property
from the STEPS entries and their type to clean up dead data, or update the
OnboardingStepDots component to render the label (e.g., next to each dot) so the
label is used—locate the STEPS array and the rendering logic in
OnboardingStepDots (and the OnboardingStep type) and apply one of these two
changes consistently.

---

23-56: Consider adding ARIA attributes for accessibility.

The step progress indicator lacks screen reader context. Adding semantic attributes would improve accessibility for users relying on assistive technologies.

♿ Suggested accessibility improvements

-    <div className="flex items-center justify-center gap-0 w-full">
+    <div 
+      className="flex items-center justify-center gap-0 w-full"
+      role="group"
+      aria-label={`Onboarding progress: step ${currentIdx + 1} of ${STEPS.length}`}
+    >
       {STEPS.map((step, i) => {
         const isCompleted = i < currentIdx;
         const isCurrent = i === currentIdx;

         return (
-          <div key={step.id} className="flex items-center">
+          <div 
+            key={step.id} 
+            className="flex items-center"
+            aria-current={isCurrent ? "step" : undefined}
+          >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingStepDots.tsx` around lines 23 - 56, Update
the OnboardingStepDots component to expose semantic ARIA attributes: add
role="list" and an appropriate aria-label (e.g., "Onboarding progress") to the
outer container, set each step wrapper (the div keyed by step.id) to
role="listitem", and on the dot element include aria-current="step" when
isCurrent and a clear accessible name (e.g., the step.title/step.label via a
visually-hidden span or aria-label) so screen readers know which step is current
and what each dot represents; also mark future steps as aria-disabled="true" or
omit aria-current for non-current steps. Ensure you update the JSX near
STEPS.map, currentIdx, and the dot/container divs, using cn only for visual
classes.

app/api/account/update/route.tsx (1)

9-9: Consider adding type safety for the request body.

The destructured fields from body are implicitly typed as any. While this works, adding a TypeScript interface for the expected request payload would improve type safety and make the API contract explicit.

🔧 Suggested type definition

interface UpdateAccountRequest {
  accountId: string;
  instruction?: string;
  name?: string;
  organization?: string;
  image?: string;
  jobTitle?: string;
  roleType?: string;
  companyName?: string;
  knowledges?: unknown;
  onboardingStatus?: Record<string, unknown>;
  onboardingData?: Record<string, unknown>;
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@app/api/account/update/route.tsx` at line 9, Create a TypeScript interface
(e.g. UpdateAccountRequest) that declares accountId: string and the optional
fields instruction, name, organization, image, jobTitle, roleType, companyName,
knowledges, onboardingStatus, and onboardingData with appropriate types (use
unknown or Record<string, unknown> where structure is not fixed), then annotate
the request body with that interface where you parse/destructure body (the
variable named body in the route handler) so the destructured line "const {
instruction, name, organization, accountId, image, jobTitle, roleType,
companyName, knowledges, onboardingStatus, onboardingData } = body;" is
type-checked against UpdateAccountRequest; adjust any downstream usages to
satisfy the new types.

components/Onboarding/OnboardingArtistsStep.tsx (1)

117-138: Consider using a stable key instead of array index for the artist list.

Using key={i} (line 120) can cause rendering issues if artists are reordered or removed from the middle. Since artists have unique name (with case-insensitive dedup) or spotifyUrl, consider using a composite key.

♻️ Use stable key for artist items

 {artists.map((a, i) => (
-  <li key={i} className="flex items-center gap-3 rounded-xl border bg-muted/20 px-3 py-2.5">
+  <li key={a.spotifyUrl ?? a.name.toLowerCase()} className="flex items-center gap-3 rounded-xl border bg-muted/20 px-3 py-2.5">

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingArtistsStep.tsx` around lines 117 - 138, The
artist list uses the array index as the React key (key={i}) which is unstable;
update the list rendering in the component that maps artists (the artists.map in
OnboardingArtistsStep) to use a stable composite key such as `${a.spotifyUrl ??
a.name.toLowerCase()}` or similar so keys remain stable across
reorders/removals; keep the remove(i) call the same but ensure each <li> uses
that stable identifier instead of i and reference ArtistAvatar and the a object
fields (name, spotifyUrl, imageUrl) when constructing the key.

components/Onboarding/OnboardingConnectionsStep.tsx (1)

104-119: Good loading and disabled states on the Connect button.

The button properly shows a spinner during connection and is disabled to prevent double-clicks. Consider adding aria-label to indicate the external link behavior for screen readers.

♿ Accessibility enhancement

 <Button
   size="sm"
   variant="outline"
   onClick={() => handleConnect(c.slug)}
   disabled={isConnecting}
   className="shrink-0"
+  aria-label={`Connect ${c.name} (opens in new tab)`}
 >

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingConnectionsStep.tsx` around lines 104 - 119,
The Connect button (Button) should include an accessible label indicating it
opens an external destination for screen readers; update the JSX where onClick
calls handleConnect(c.slug) and isConnecting controls state to pass an
aria-label like `Connect to ${c.slug} (opens external link)` or similar
descriptive text to the Button (or the ExternalLink icon if you prefer the label
on the icon), ensuring the label dynamically uses c.slug and preserving existing
isConnecting and disabled behavior.

components/Onboarding/OnboardingWelcomeStep.tsx (1)

119-129: Consider adding accessible labels for status indicators.

The emoji indicators (✅/⏳) are decorative but convey status information. Screen readers may not announce these meaningfully. Consider adding aria-label or aria-hidden with a visually hidden status text.

♿ Proposed accessibility improvement

 <motion.div
   key={i}
   initial={{ opacity: 0, x: -8 }}
   animate={{ opacity: 1, x: 0 }}
   className="flex items-center gap-2 text-sm"
 >
-  <span className="text-base">
+  <span className="text-base" aria-hidden="true">
     {i < lineIdx ? "✅" : "⏳"}
   </span>
-  <span className={i < lineIdx ? "text-foreground" : "text-foreground animate-pulse"}>
+  <span 
+    className={i < lineIdx ? "text-foreground" : "text-foreground animate-pulse"}
+    aria-label={i < lineIdx ? "Completed:" : "In progress:"}
+  >
     {line}
   </span>
 </motion.div>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingWelcomeStep.tsx` around lines 119 - 129, The
emoji status indicators inside the OnboardingWelcomeStep (the motion.div that
renders {i < lineIdx ? "✅" : "⏳"} alongside {line}) are decorative but convey
state; update that rendering so the emoji itself is aria-hidden and add an
accessible status label for screen readers—either by adding an aria-label on the
motion.div (e.g., "completed" vs "in progress" based on i and lineIdx) or by
inserting a visually-hidden <span> with that status text next to the emoji;
ensure the logic uses the same i and lineIdx conditions so screen readers
receive the current status for each line.

components/Onboarding/OnboardingContextStep.tsx (1)

35-47: Consider the stale closure risk in useEffect dependencies.

The effect intentionally skips name and onChangeName to avoid re-running after manual edits, but this pattern can cause subtle bugs if the parent re-renders with different callback references. A safer approach is to use a ref to track whether pre-fill has occurred.

♻️ Alternative using a ref to track pre-fill state

+import { useEffect, useRef } from "react";
 
 export function OnboardingContextStep({
   ...
 }: Props) {
   const { userData, email } = useUserProvider();
+  const hasPrefilled = useRef(false);
 
   useEffect(() => {
-    if (!name && (userData?.name || email)) {
+    if (!hasPrefilled.current && !name && (userData?.name || email)) {
+      hasPrefilled.current = true;
       const inferred =
         userData?.name ||
         (email
           ? email
               .split("@")[0]
               .replace(/[._]/g, " ")
               .replace(/\b\w/g, c => c.toUpperCase())
           : "");
       if (inferred) onChangeName(inferred);
     }
-  }, [userData?.name, email]); // eslint-disable-line react-hooks/exhaustive-deps
+  }, [userData?.name, email, name, onChangeName]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingContextStep.tsx` around lines 35 - 47,
Replace the current useEffect with a version that uses a ref (e.g.,
hasPrefilledRef) to ensure pre-fill runs only once and avoid the stale-closure
risk: create const hasPrefilledRef = useRef(false), then in the effect check if
(!hasPrefilledRef.current && !name && (userData?.name || email)) { /* compute
inferred same as before */ if (inferred) { onChangeName(inferred);
hasPrefilledRef.current = true } }, and include onChangeName, userData?.name,
email (and optionally name) in the dependency array and remove the
eslint-disable comment so the effect sees updated callback references.

components/Onboarding/OnboardingCompleteStep.tsx (2)

97-98: Hide decorative emoji from assistive technologies.

Line 97 is decorative and duplicates Line 98 semantics; mark it aria-hidden for cleaner screen-reader output.

Suggested fix

-                <span className="text-lg">{item.icon}</span>
+                <span className="text-lg" aria-hidden="true">{item.icon}</span>
                 <span className="font-medium">{item.text}</span>

As per coding guidelines, **/*.{tsx,ts,jsx,js}: "Provide proper ARIA roles/states and test with screen readers".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingCompleteStep.tsx` around lines 97 - 98, The
decorative emoji span in the OnboardingCompleteStep component (the <span>
rendering item.icon alongside item.text) duplicates the textual semantics and
should be hidden from assistive tech; update the icon span that renders
item.icon to include aria-hidden="true" (or aria-hidden={true}) so screen
readers ignore it while leaving the text span (item.text) unchanged.

---

8-25: Use an explicit exported props type name for the component API.

Replace Props with OnboardingCompleteStepProps and export it for clearer, consistent typing.

Suggested fix

-interface Props {
+export interface OnboardingCompleteStepProps {
   artistNames: string[];
   name: string | undefined;
   connectedCount: number;
   pulseEnabled: boolean;
   onComplete: () => void;
 }
@@
 export function OnboardingCompleteStep({
   artistNames,
   name,
   connectedCount,
   pulseEnabled,
   onComplete,
-}: Props) {
+}: OnboardingCompleteStepProps) {

As per coding guidelines, **/*.{tsx,ts}: "Export component prop types with explicit ComponentNameProps naming convention".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingCompleteStep.tsx` around lines 8 - 25, Rename
the inline Props type to an exported, explicitly named type
OnboardingCompleteStepProps and update the component signature to use that type:
export type OnboardingCompleteStepProps = { artistNames: string[]; name: string
| undefined; connectedCount: number; pulseEnabled: boolean; onComplete: () =>
void; } and change the function parameter typing in OnboardingCompleteStep to
accept OnboardingCompleteStepProps instead of Props; ensure the new type is
exported so other modules can import it.

components/Onboarding/OnboardingCompleteStep.tsx (2)

34-51: Consider extracting summary item generation for testability.

The summaryItems construction logic is inline and uses a filter-then-cast pattern. While functional, extracting this to a pure function would improve testability and make the conditional logic more explicit.

♻️ Optional: Extract summary builder

+type SummaryItem = { icon: string; text: string };
+
+function buildSummaryItems(
+  artistNames: string[],
+  connectedCount: number,
+  pulseEnabled: boolean,
+): SummaryItem[] {
+  const items: SummaryItem[] = [];
+  if (artistNames.length > 0) {
+    items.push({
+      icon: "🎤",
+      text: `Deep research running on ${artistNames.slice(0, 2).join(" & ")}${
+        artistNames.length > 2 ? ` +${artistNames.length - 2} more` : ""
+      }`,
+    });
+  }
+  if (connectedCount > 0) {
+    items.push({
+      icon: "🔗",
+      text: `${connectedCount} platform${connectedCount > 1 ? "s" : ""} connected`,
+    });
+  }
+  if (pulseEnabled) {
+    items.push({ icon: "⚡", text: "Pulse active — briefing arrives tomorrow morning" });
+  }
+  items.push(
+    { icon: "✅", text: "First week of tasks queued" },
+    { icon: "🧠", text: "AI learning your artists and fans right now" },
+  );
+  return items;
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingCompleteStep.tsx` around lines 34 - 51,
Extract the inline summaryItems logic into a pure function (e.g.,
buildSummaryItems or getSummaryItems) that accepts artistNames: string[],
connectedCount: number, and pulseEnabled: boolean and returns a typed array {
icon: string; text: string }[]; move the conditional item creation out of the
JSX and into that function, avoid the filter(Boolean) + cast pattern by only
pushing valid items to the result array or using a compact map/filter with
proper typing, then replace the inline summaryItems with a call to the new
function so the logic is testable and isolated.

---

58-63: Minor indentation inconsistency in JSX.

The motion.div starting at line 58 has inconsistent indentation compared to its parent structure. While this doesn't affect functionality, consistent formatting improves readability.

🔧 Fix indentation

         <>
           <OnboardingConfetti />
           <motion.div
-          className="flex flex-col items-center gap-7 text-center"
-          initial={{ opacity: 0, scale: 0.95 }}
-          animate={{ opacity: 1, scale: 1 }}
-          transition={{ duration: 0.4, ease: "easeOut" }}
-        >
+            className="flex flex-col items-center gap-7 text-center"
+            initial={{ opacity: 0, scale: 0.95 }}
+            animate={{ opacity: 1, scale: 1 }}
+            transition={{ duration: 0.4, ease: "easeOut" }}
+          >

Also applies to: 126-126

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingCompleteStep.tsx` around lines 58 - 63, The
JSX indentation for the motion.div elements in OnboardingCompleteStep (the
<motion.div ...> blocks) is inconsistent with the surrounding JSX; fix by
aligning those motion.div opening and closing tags and their props to the same
indentation level as their sibling JSX elements inside the component (ensure the
className/initial/animate/transition props are indented one level in from the
parent element), and make the same adjustment for the other occurrence of the
motion.div to maintain consistent formatting throughout the component.

components/Onboarding/OnboardingModal.tsx (1)

39-39: Extract magic number to a named constant.

The 200 millisecond delay is a magic number. Extracting it to a named constant improves readability and makes the intent clearer.

♻️ Extract constant

+const REDIRECT_DELAY_MS = 200;
+
 const handleComplete = useCallback(async () => {
   // ...
-  setTimeout(() => { window.location.href = `/?q=${q}`; }, 200);
+  setTimeout(() => { window.location.href = `/?q=${q}`; }, REDIRECT_DELAY_MS);
 }, [complete, persist, data]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingModal.tsx` at line 39, Extract the magic
number 200ms used in the setTimeout inside the OnboardingModal component into a
named constant (e.g., ONBOARDING_REDIRECT_DELAY_MS or REDIRECT_DELAY_MS)
declared at the top of the module (or near the OnboardingModal definition) and
replace the inline literal in setTimeout(() => { window.location.href =
`/?q=${q}`; }, 200) with that constant so the delay is self-documenting and easy
to adjust.

components/Onboarding/OnboardingConnectionsStep.tsx (1)

61-69: ⚠️ Potential issue | 🟠 Major

Closing the auth window still marks the connector as connected.

Manual close or a canceled OAuth flow will still hit onConnect(slug), so the checkmark is still optimistic. Please only mark the slug connected from a verified callback or a backend status check after the OAuth flow completes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@components/Onboarding/OnboardingConnectionsStep.tsx` around lines 61 - 69,
The current logic in the popup OAuth flow unconditionally calls onConnect(slug)
when the popup is closed (see window.open, popup, checkClosed, onConnect,
setConnecting), which marks connectors connected even on user-cancel or failed
OAuth; change this to only mark a connector connected after a confirmed backend
callback or verified status: remove the onConnect(slug) invocation from the
popup.closed branch and instead implement a verification step (e.g., poll a
backend endpoint or listen for a postMessage/webhook confirmation) that checks
connection status for the given slug and only call onConnect(slug) and
setConnecting(null) after that verification succeeds, while ensuring the popup
closed case cancels the spinner and does not optimistically mark the connection.